fix: remove separate locator results scrollbar for mobile#1237
Conversation
In mobile view, display a "show more" button insteas of pagination. J=WAT-5580 TEST=manual tested in dev mode
|
Warning: Component files have been updated but no migrations have been added. See https://github.com/yext/visual-editor/blob/main/packages/visual-editor/src/components/migrations/README.md for more information. |
commit: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
WalkthroughThis PR centralizes viewport breakpoint logic into a new hook module (VIEWPORT_BREAKPOINTS, getViewport, useWindowWidth), migrates header components to those shared utilities, and refactors Locator to derive searchResults from headless state, detect mobile by preview window width, accumulate mobileResults for "Show more locations" pagination, add a MobileLocatorResultsSection for mobile rendering, and avoid desktop scroll-to-top when on mobile. Multiple locale JSON files were updated to add the showMoreLocations key. Sequence Diagram(s)sequenceDiagram
participant User
participant LocatorComponent
participant HeadlessSearchState
participant MobileLocatorResultsSection
User->>LocatorComponent: Perform search / open locator
LocatorComponent->>HeadlessSearchState: Read state.vertical.results
LocatorComponent->>LocatorComponent: Determine isMobile via preview window width -> getViewport
alt Mobile viewport
LocatorComponent->>MobileLocatorResultsSection: Render mobile cards + "Show more locations"
User->>MobileLocatorResultsSection: Click "Show more locations"
MobileLocatorResultsSection->>LocatorComponent: request offset increment
LocatorComponent->>HeadlessSearchState: set offset / fetch next results
HeadlessSearchState->>LocatorComponent: return new results
LocatorComponent->>LocatorComponent: append new results to mobileResults
LocatorComponent->>MobileLocatorResultsSection: render appended list
else Desktop viewport
LocatorComponent->>LocatorComponent: Render VerticalResults and handle desktop scroll-to-top on offset change
end
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/visual-editor/src/components/Locator.tsx`:
- Around line 1729-1739: When entering mobile viewport with a non-zero
currentOffset the existing useEffect appends only the current page
(setMobileResults), which causes mobileResults.length to be an incorrect offset
for subsequent "Show more" requests; modify the effect that depends on
currentOffset/isMobileViewport/searchResults to detect the transition into
mobile (isMobileViewport true when previously false) or a non-zero currentOffset
and then either reset mobileResults to an empty array and set offset to 0, or
rebuild mobileResults as the full prefix of pages up to currentOffset by
aggregating past pages (using currentOffset, searchResults and any stored page
history) before appending the current page, so that mobileResults.length
accurately reflects the next offset used by the "Show more" logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 600c01b8-d9ca-41d0-b65e-9f012cc1332b
📒 Files selected for processing (5)
packages/visual-editor/src/components/Locator.tsxpackages/visual-editor/src/components/header/HeaderLinks.tsxpackages/visual-editor/src/components/header/PrimaryHeaderSlot.tsxpackages/visual-editor/src/components/header/viewport.tspackages/visual-editor/src/hooks/useViewport.ts
💤 Files with no reviewable changes (1)
- packages/visual-editor/src/components/header/viewport.ts
vijay267
left a comment
There was a problem hiding this comment.
LGTM, but will defer to Sumo as well
…tor sees correct viewport on first mount
jwartofsky-yext
left a comment
There was a problem hiding this comment.
Looks good to me
I would like to reiterate that I think the Locator.tsx file should be broken up a bit as it is over 2k lines long
It would be reasonable to make a Locator/ folder with different files for Locator.tsx, Card.tsx, ResultsCounts.tsx, etc
auto-screenshot-update: true
I agree. Created an item dedicated for this so we don't postpone forever, but I'll close this PR out as is |
In mobile view, display a "show more" button at the end of the search results instead of pagination.
J=WAT-5580
TEST=manual
tested in dev mode and platform
https://drive.google.com/file/d/1Bw7xFbmvdOncZl37DXCalcTiLl3zod0n/view?usp=sharing